Skip to content

Add ClaimableAwaitingConfirmations::BalanceSource#138

Merged
tnull merged 1 commit intolightningdevkit:mainfrom
tankyleo:2026-03-balance-source
Mar 3, 2026
Merged

Add ClaimableAwaitingConfirmations::BalanceSource#138
tnull merged 1 commit intolightningdevkit:mainfrom
tankyleo:2026-03-balance-source

Conversation

@tankyleo
Copy link
Contributor

@tankyleo tankyleo commented Mar 2, 2026

No description provided.

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Mar 2, 2026

👋 Thanks for assigning @tnull as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@tankyleo tankyleo requested a review from benthecarman March 2, 2026 06:35
@tankyleo tankyleo force-pushed the 2026-03-balance-source branch from 5b596ee to fb4ff8f Compare March 2, 2026 06:54
Copy link
Collaborator

@tnull tnull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I'm missing something, but AFAICT we should be able to drop those custom implementations for SCREAMING_SNAKE_CASE?

Htlc = 3,
}

impl BalanceSource {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIU, there is no need for this custom impl, serde has SCREAMING_SNAKE_CASE.

/// Indicates whether the balance is derived from a cooperative close, a force-close (for holder or counterparty),
/// or whether it is for an HTLC.
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
#[cfg_attr(feature = "serde", serde(rename_all = "snake_case"))]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#[cfg_attr(feature = "serde", serde(rename_all = "snake_case"))]
#[cfg_attr(feature = "serde", serde(rename_all = "SCREAMING_SNAKE_CASE"))]

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is auto generated so idk if we can really edit these files

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right.

Copy link
Collaborator

@benthecarman benthecarman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@tankyleo tankyleo requested review from tnull and removed request for tnull March 2, 2026 21:42
Copy link
Collaborator

@tnull tnull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Experimented a bit with alternate approached but didn't get to a materially different point right now. Will go ahead and land this.

@tnull tnull merged commit 137e571 into lightningdevkit:main Mar 3, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants